Allow for globs in `probe.resolvers.intellij.repositories` config #285

LukaszKontowski · 2022-09-26T11:38:37Z

Closes #250
work in progress

to be done:

add description and some example for users in reference.conf
add one more test scenario to test new logic with multiple paths (but only one is valid)

PawelLipski · 2022-09-26T13:14:35Z

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

+  private def replaceFirstFoundWildcardWithDirectories(path: String): List[String] = {
+    def removeLastFileSeparator(path: String): String = if (path.endsWith("/")) path.init else path
+
+    val pathUntilWildcard = Paths.get(path.substring(0, path.indexOf('*')).replace("file:", ""))


Okay... so a ready library for globs wasn't an adequate solution here? 🤔

I did not manage to test these libraries yet. Wanted to have a working solution first. I'm not sure if we need them here. Suggested docs https://docs.oracle.com/javase/tutorial/essential/io/find.html don't seem helpful to me in this case.

Ok, leaving up to your judgement

core/driver/sources/src/main/resources/reference.conf

PawelLipski · 2022-09-27T19:12:41Z

core/driver/sources/src/main/resources/reference.conf

@@ -109,6 +109,21 @@ probe {
    // ~/.pluginVerifier/ides/IC-[revision]/ directory (if exists). If it does not exist under specified path,
    // then ide-probe will download it from one of the 6 official repositories.
    //
+    // If your config contains a pattern pointing to an IntelliJ instance that exists on local filesystem, then you


:benissimo: the doc

Co-authored-by: Paweł Lipski <[email protected]>

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

lukaszwawrzyk · 2022-10-11T13:31:03Z

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

+
+  private def pathMightBeValidResource(path: String): Boolean =
+    path.indexOf('*') == -1 &&
+      (path.endsWith(".zip") ||


isn't it too specific check? Filling extension is responsibility of pattern replacement that already happened.

Yes, the check is needed to find proper path from multiple directories that exist in place of * (some are not valid).
But I just realised that this check is not valid. This check should return true only when path ends properly, as defined in the pattern... Let me fix this.

Ok, logic of pathMightBeValidResource method is fixed now.

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

lukaszwawrzyk · 2022-10-11T13:33:49Z

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

+    def removeLastFileSeparator(path: String): String = if (path.endsWith("/")) path.init else path
+
+    val pathUntilWildcard = Paths.get(path.substring(0, path.indexOf('*')).replace("file:", ""))
+    val stringPathAfterWildcard = path.substring(path.indexOf('*') + 1)


can you use split for that? It allows to specify limit of splits.

Ok, but then I would have to use three vals instead of two:

val Array(stringPathUntilWildcard, stringPathAfterWildcard) = path.split("*", 2) val pathUntilWildcard = Paths.get(stringPathUntilWildcard.replace("file:", ""))

If you think that would be better than current solution, I can do that. Let me know, what do you think.

Yes, let's do this. It is same number of lines, but 2 substring calls with some index offset are more complex for reading and understanding than a simple split.

There is a problem connected to replacing String#indexOf with String#split
The String#split(String regex, int limit) method treats the String parameter "*" as a regular expression, which leads to invalid results.

I would suggest to switch back to previous logic:

val pathUntilWildcard = Paths.get(path.substring(0, path.indexOf('*')).replace("file:", "")) val stringPathAfterWildcard = path.substring(path.indexOf('*') + 1)

to be sure that * wildcard will not be used as a regular expression.

To be sure, just use regex as expected r"\*" or "[*]"

Ok, but the thing is - I don't want java String's method to treat the * as a regular expression. And String#split by default treats the String parameter as a regular expression. That's why I want to stick to current logic, without using split. So that * will never be treated as a regex.

…com:LukaszKontowski/ide-probe into feature/enable_globs_in_repositories_config

This reverts commit c6c43a4.

LukaszKontowski · 2022-10-18T08:23:20Z

Added two empty files to test resources so that resolvesIntelliJPatternWithGlobesUsed test will check if our logic works properly - chooses the proper file and not one of two invalid paths.

PawelLipski · 2022-10-18T11:07:07Z

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala

+
+  // Solution below assumes that each * character is used to mark one part of the path (one atomic directory),
+  // for example: "file:///home/.cache/ides/com.jetbrains.intellij.idea/ideaIC/[revision]/*/ideaIC-[revision]/".
+  // Wildcard character should NOT be used as the last element of the pattern - to avoid ambiguous results.


Hmmm technically, ambiguous results can also happen when wildcard is somewhere inside the path as well, right?

Maybe sth like:

Suggested change

// Wildcard character should NOT be used as the last element of the pattern - to avoid ambiguous results.

// Wildcard character should NOT be used as the last element of the pattern - to decrease the probability of getting ambiguous results.

ambiguous results can also happen when wildcard is somewhere inside the path as well, right?

No, if the * wildcard is used in replacement of an atomic directory from the path (but not the last part) - then

private def resolveGlobsInPattern(pathPattern: String): String = replaceGlobsWithExistingDirectories(List(pathPattern), pathPattern).head

will return only one element if pattern is valid (so the result will not be ambiguous)
or

will throw NoSuchElementException if pattern is not valid

Huh okay... so let's break down all cases:

zero glob matches: exception regardless of where the glob is located?

exactly one glob match: OK

more than one glob match:
a. if * is used for the last part of the glob — multiple results returned?
b. if * is used for other part of the glob — exception thrown?

Answered in PMs but short answers below:

zero glob matches: exception regardless of where the glob is located?

NoSuchElementException will be thrown (as pattern is not valid - such file does not exist)

exactly one glob match: OK

yes

more than one glob match:
a. if * is used for the last part of the glob — multiple results returned?

no - NoSuchElementException will be thrown - using * as the last element of the path is not possible, because then the logic would not be able to choose the proper path.
For example - if we would have path endings like below:

.../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT .../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.zip .../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.jar .../df364b7662aeb20ca2eafead730380017c395661/ideaIC-223.4884.69-EAP-SNAPSHOT.pom

where first one is directory and second one - a .zip file - and the pattern ends with * - how can we know, which one should be used?

more than one glob match:
b. if * is used for other part of the glob — exception thrown?

no - resolveGlobsInPattern will choose the first one by calling .head

LukaszKontowski added 2 commits September 26, 2022 11:47

Allow for globs in probe.resolvers.intellij.repositories config

0cf9f75

Fix globes logic and allow for multiple globes in one pattern

4a5ef3b

LukaszKontowski self-assigned this Sep 26, 2022

This was referenced Sep 26, 2022

Draft: Feature/allow for globs repositories config #284

Closed

Use intellij instances downloaded by gradle VirtusLab/git-machete-intellij-plugin#1158

Merged

LukaszKontowski changed the title ~~Feature/enable globs in repositories config~~ Allow for globs in probe.resolvers.intellij.repositories config Sep 26, 2022

PawelLipski reviewed Sep 26, 2022

View reviewed changes

Add example with description in reference.conf

aab9a1c

LukaszKontowski marked this pull request as ready for review September 27, 2022 16:16

LukaszKontowski requested a review from PawelLipski September 27, 2022 16:16

PawelLipski reviewed Sep 27, 2022

View reviewed changes

core/driver/sources/src/main/resources/reference.conf Outdated Show resolved Hide resolved

PawelLipski reviewed Sep 27, 2022

View reviewed changes

PawelLipski approved these changes Sep 27, 2022

View reviewed changes

Fix misspell in reference.conf description

88413db

Co-authored-by: Paweł Lipski <[email protected]>

lukaszwawrzyk reviewed Oct 11, 2022

View reviewed changes

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala Outdated Show resolved Hide resolved

lukaszwawrzyk reviewed Oct 11, 2022

View reviewed changes

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala Outdated Show resolved Hide resolved

lukaszwawrzyk reviewed Oct 11, 2022

View reviewed changes

core/driver/sources/src/main/scala/org/virtuslab/ideprobe/dependencies/IntelliJResolver.scala Outdated Show resolved Hide resolved

lukaszwawrzyk reviewed Oct 11, 2022

View reviewed changes

LukaszKontowski marked this pull request as draft October 14, 2022 15:30

This comment was marked as outdated.

Sign in to view

LukaszKontowski added 3 commits October 17, 2022 14:33

PR fixes - fix resolving logic and improve code style

8319647

Merge branch 'feature/enable_globs_in_repositories_config' of github.…

cf8b83f

…com:LukaszKontowski/ide-probe into feature/enable_globs_in_repositories_config

Fi logic for checking las part of the candidate path

b5f6fa7

LukaszKontowski marked this pull request as ready for review October 17, 2022 13:26

LukaszKontowski added 3 commits October 17, 2022 16:03

fix logic for non-local patterns

b7f601c

Improve code style - use String#split

c6c43a4

Revert "Improve code style - use String#split"

93ce1ec

This reverts commit c6c43a4.

LukaszKontowski marked this pull request as draft October 17, 2022 15:39

Improve test scanarios

dd834df

LukaszKontowski marked this pull request as ready for review October 17, 2022 20:55

LukaszKontowski requested review from lukaszwawrzyk and PawelLipski October 17, 2022 20:55

PawelLipski reviewed Oct 18, 2022

View reviewed changes

lukaszwawrzyk approved these changes Oct 19, 2022

View reviewed changes

LukaszKontowski requested a review from lukaszwawrzyk October 19, 2022 11:30

LukaszKontowski merged commit 3cdb91e into VirtusLab:master Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for globs in `probe.resolvers.intellij.repositories` config #285

Allow for globs in `probe.resolvers.intellij.repositories` config #285

LukaszKontowski commented Sep 26, 2022 •

edited

Loading

PawelLipski Sep 26, 2022

LukaszKontowski Sep 26, 2022

PawelLipski Sep 26, 2022

PawelLipski Sep 27, 2022

lukaszwawrzyk Oct 11, 2022

LukaszKontowski Oct 14, 2022

LukaszKontowski Oct 17, 2022

lukaszwawrzyk Oct 11, 2022

LukaszKontowski Oct 17, 2022 •

edited

Loading

lukaszwawrzyk Oct 17, 2022

LukaszKontowski Oct 17, 2022

lukaszwawrzyk Oct 19, 2022 •

edited

Loading

LukaszKontowski Oct 19, 2022

This comment was marked as outdated.

LukaszKontowski commented Oct 18, 2022 •

edited

Loading

PawelLipski Oct 18, 2022

LukaszKontowski Oct 18, 2022

PawelLipski Oct 18, 2022

LukaszKontowski Oct 18, 2022 •

edited

Loading

	// Wildcard character should NOT be used as the last element of the pattern - to avoid ambiguous results.
	// Wildcard character should NOT be used as the last element of the pattern - to decrease the probability of getting ambiguous results.

Allow for globs in probe.resolvers.intellij.repositories config #285

Allow for globs in probe.resolvers.intellij.repositories config #285

Conversation

LukaszKontowski commented Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukaszKontowski Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukaszwawrzyk Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

LukaszKontowski commented Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukaszKontowski Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

Allow for globs in `probe.resolvers.intellij.repositories` config #285

Allow for globs in `probe.resolvers.intellij.repositories` config #285

LukaszKontowski commented Sep 26, 2022 •

edited

Loading

LukaszKontowski Oct 17, 2022 •

edited

Loading

lukaszwawrzyk Oct 19, 2022 •

edited

Loading

LukaszKontowski commented Oct 18, 2022 •

edited

Loading

LukaszKontowski Oct 18, 2022 •

edited

Loading